Replace plugins package with client-common #72
Conversation
|
If one of the three separated plugins are not installed, then any configuration that specifically enables those plugins will end up in a BC break. So I think we have to install the plugins package as well, and replace the used class if the package is installed. |
| @@ -19,6 +19,7 @@ | |||
| "php": ">=5.5", | |||
| "php-http/client-implementation": "^1.0", | |||
| "php-http/message-factory": "^1.0.2", | |||
| "php-http/client-common": "^1.1", | |||
| "php-http/plugins": "^1.0", | |||
There was a problem hiding this comment.
if we require both the new and the old plugins now, we don't need the flexible code you introduce above.
but i think to avoid BC breaks, we can indeed depend on both.
There was a problem hiding this comment.
ah sorry, i was confused. what you do is for the plugins that are now separate you use the new one if are available and otherwise keep using the old one. i think that is fine. once we move to 2.0 the separate plugins will only be available if included.
|
looks good. i suggest we bump the composer alias for master to 1.1-dev so that we release this as a new minor version. together with the other fix i started preparing. |
|
Why not 1.2-dev then? |
|
Squashed commits. Anything else here @dbu? |
| @@ -137,6 +138,9 @@ private function configurePluginByName($name, Definition $definition, array $con | |||
| { | |||
| switch ($name) { | |||
| case 'cache': | |||
| if (class_exists('Http\Client\Common\Plugin\CachePlugin')) { | |||
There was a problem hiding this comment.
I would prefer a comment here that says why your are doing this. It seams kind of weird if you do not know that one namespace was deprecated and you are doing this because we want to keep BC.
Same with the logger and stopwatch.
There was a problem hiding this comment.
Great idea, thanks, will do that.
sagikazarmark
force-pushed
the
plugins
branch
2 times, most recently
from
561d2df to
de3388c
Compare
|
👍 |
|
|
||
| - **[BC] PluginClientFactory returns an instance of `Http\Client\Common\PluginClient`** (see [php-http/client-common#14](https://github.com/php-http/client-common/pull/14)) | ||
| - Plugins are loaded from their new packages, fall back to `php-http/plugins` if necessary | ||
|
|
There was a problem hiding this comment.
should we say that version 2 of the bundle will not automatically require the plugins that moved to separate repositories? i am not sure if we want to do that actually - cache, logger and stopwatch all depend on things that symfony depends on anyways, so there is no cost in requiring them in the bundle, right? maybe we even want to explicitly require those new plugins rather than the BC repository.
There was a problem hiding this comment.
I am fine with that as well. In that case, we don't need to rely on the old package.
|
needs a rebase |
Applied fixes from StyleCI Add plugins dependency back as fallback Remove auto (default) config from plugins Update changelog Make sure to use a plugin version with correct interface
|
Added a separate commit for plugin package removal. Blocked by php-http/stopwatch-plugin#1 |
|
https://github.com/php-http/stopwatch-plugin/releases/tag/1.0.1 i restarted the build, lets see if its fixed with this |
|
Dont forget to set the lowest version for the plugin. |
|
do you mean stopwatch? composer will figure that out i think. |
|
Symfony 3 will not work with 1.0 as the lowest version David Buchmann notifications@github.com ezt írta (2016. május 19.,
|
|
composer will simply choose 1.0.1 with symfony 3, as there is no other
resolution to satisfy all constraint.
|
|
Ok |
|
Merge and tag 1.1? |
|
yup, done. thanks!
|
What's in this PR?
Replaces php-http/plugins with php-http/common-client.
Why?
The plugins package has been deprecated and moved to client-common.
Checklist